Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

fix: remove redundant Cloudflare worker asset check #9040

Merged
merged 1 commit into from
Feb 14, 2023
Merged

fix: remove redundant Cloudflare worker asset check #9040

merged 1 commit into from
Feb 14, 2023

Conversation

eltigerchino
Copy link
Member

@eltigerchino eltigerchino commented Feb 14, 2023 •
edited
Loading

closes #9039

Removes the generated file check in the worker and adds the no-cache header value for /_app/version.json in the generated headers.

Please don't delete this checklist! Before submitting the PR, please make sure you do the following:

  • It's really useful if your PR references an issue where it is discussed ahead of time. In many cases, features are absent for a reason. For large changes, please create an RFC: https://github.com/sveltejs/rfcs
  • This message body should clearly illustrate what problems it solves.
  • Ideally, include a test that fails without this PR but passes with it.

Tests

  • Run the tests with pnpm test and lint the project with pnpm lint and pnpm check

Changesets

  • If your PR makes a change that should be noted in one or more packages' changelogs, generate a changeset by running pnpm changeset and following the prompts. Changesets that add features should be minor and those that fix bugs should be patch. Please prefix changeset messages with feat:, fix:, or chore:.

@Rich-Harris Rich-Harris merged commit fdcfb87 into sveltejs:master Feb 14, 2023
@github-actions github-actions bot mentioned this pull request Feb 14, 2023
Merged
@TimoWilhelm
Copy link
Contributor

TimoWilhelm commented Feb 17, 2023 •
edited
Loading

Hi @s3812497 cloudflare-adapter@2.0.2 seems to have broken the deployment for me and some others (Svelte Discord Thread).

While local preview looks fine, some assets are not served anymore when accessing from a Cloudflare Pages deployment.

image

Previously, a lot of assets were returned immediately without any additional checks. Could this be the issue?
https://github.com/s3812497/kit/blob/32cbe071bb3fe5093c3946a39507f8b04a9eb737/packages/adapter-cloudflare/src/worker.js#L22-L38

@eltigerchino
Copy link
Member Author

Hi @s3812497 cloudflare-adapter@2.0.2 seems to have broken the deployment for me and some others (Svelte Discord Thread).

While local preview looks fine, some assets are not served anymore when accessing from a Cloudflare Pages deployment.

image

Previously, a lot of assets were returned immediately without any additional checks. Could this be the issue?

https://github.com/s3812497/kit/blob/32cbe071bb3fe5093c3946a39507f8b04a9eb737/packages/adapter-cloudflare/src/worker.js#L22-L38

I'm terribly sorry for that! I'll investigate more as soon as I can. Serves me right for thinking the static requests would be served just fine through the _routes.json file

@amorfati254
Copy link

I am facing the same issue as @TimoWilhelm. Worked fine 2 days a go but not working today. Works in dev and preview after building but not in cloudflare.

@eltigerchino eltigerchino deleted the fix-cloudflare-worker-asset-cache branch November 2, 2023 16:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Rich-Harris Rich-Harris Rich-Harris approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Redundant generated file check in the Cloudflare worker
4 participants
@eltigerchino @TimoWilhelm @amorfati254 @Rich-Harris